Skip to content

Update magnifier Point at start#19780

Merged
seanbudd merged 8 commits intonvaccess:masterfrom
France-Travail:fix/NVDAPythonConsole
Apr 16, 2026
Merged

Update magnifier Point at start#19780
seanbudd merged 8 commits intonvaccess:masterfrom
France-Travail:fix/NVDAPythonConsole

Conversation

@Boumtchack
Copy link
Copy Markdown
Contributor

@Boumtchack Boumtchack commented Mar 11, 2026 •
edited
Loading

Link to issue number:

fixes #19746

Summary of the issue:

Text caret follow would fail in some text editor (python console of nvda or notepad), if it would be the last character

Description of user facing changes:

text caret will now focus properly end character

Description of developer facing changes:

Description of development approach:

Update _get_pointAtStart now falls back to the previous character's position when the caret is at end-of-text and both _getBoundingRectFromOffset and _getPointFromOffset fail for the current offset.

Testing strategy:

Manual

Known issues with pull request:

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@Boumtchack Boumtchack marked this pull request as ready for review March 11, 2026 13:46
@Boumtchack Boumtchack requested a review from a team as a code owner March 11, 2026 13:46
Copilot AI reviewed Mar 11, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes magnifier caret-follow failures when the caret is positioned at end-of-text by improving how OffsetsTextInfo.pointAtStart is computed when the current offset can’t be mapped to screen coordinates.

Changes:

  • Extend _get_pointAtStart to handle LookupError as well as NotImplementedError when resolving a point for the current offset.
  • Add a fallback for collapsed ranges to use the previous character’s offset when the caret is at end-of-text and current-offset location lookup fails.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/textInfos/offsets.py Outdated
Comment thread source/textInfos/offsets.py Outdated
@CyrilleB79
Copy link
Copy Markdown
Contributor

CyrilleB79 commented Mar 11, 2026

I am not an expert of this code.

But I wonder if it would not be better, if possible, to implement the fallback in magnifier files rather than in offset.py. Indeed an offset function used elsewhere returning an erroneous information could have side effect; couldn't it?

@LeonarddeR
Copy link
Copy Markdown
Collaborator

LeonarddeR commented Mar 12, 2026

I think I agree with @CyrilleB79 here.
Note that the end offset is exclusive in text controls. So when you have a text control containing one character, that one character is at offset 0. The story length is 1 and collapsing at end is offset 1, yet offset 1 is exclusive to the text.
What would make sense to me though is returning the top right of the rectangle at the previous offset.

seanbudd
seanbudd reviewed Mar 16, 2026
View reviewed changes
Comment thread source/textInfos/offsets.py Outdated
Comment thread source/textInfos/offsets.py Outdated
@seanbudd seanbudd marked this pull request as draft March 16, 2026 05:49
@seanbudd seanbudd changed the title Update Point at start Update magnifier Point at start Mar 17, 2026
@seanbudd seanbudd added the merge-early Merge Early in a developer cycle label Mar 17, 2026
@seanbudd seanbudd self-requested a review March 24, 2026 04:25
@SaschaCowley SaschaCowley added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Mar 24, 2026
@seanbudd
Copy link
Copy Markdown
Member

seanbudd commented Mar 24, 2026

Is this ready for re-review?

@Boumtchack Boumtchack marked this pull request as ready for review March 30, 2026 13:48
seanbudd
seanbudd reviewed Mar 31, 2026
View reviewed changes
Comment thread source/_magnifier/utils/focusManager.py Outdated
Comment thread source/_magnifier/utils/focusManager.py Outdated
Comment thread source/_magnifier/utils/focusManager.py Outdated
Comment thread source/_magnifier/utils/focusManager.py Outdated
Comment thread source/_magnifier/utils/focusManager.py Outdated
@seanbudd seanbudd marked this pull request as draft March 31, 2026 02:12
@Boumtchack Boumtchack marked this pull request as ready for review March 31, 2026 12:18
seanbudd
seanbudd reviewed Apr 1, 2026
View reviewed changes
Comment thread source/_magnifier/utils/focusManager.py
seanbudd
seanbudd reviewed Apr 1, 2026
View reviewed changes
Comment thread source/_magnifier/utils/focusManager.py
Comment thread source/_magnifier/utils/focusManager.py
@seanbudd seanbudd marked this pull request as draft April 1, 2026 02:57
This was referenced Apr 7, 2026
Closed
Closed
@Boumtchack Boumtchack marked this pull request as ready for review April 14, 2026 14:10
seanbudd
seanbudd approved these changes Apr 16, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Boumtchack

@seanbudd seanbudd merged commit efe3ebe into nvaccess:master Apr 16, 2026
39 of 41 checks passed
@github-actions github-actions bot added this to the 2026.2 milestone Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@seanbudd seanbudd seanbudd approved these changes

@SaschaCowley SaschaCowley Awaiting requested review from SaschaCowley SaschaCowley is a code owner automatically assigned from nvaccess/developers

Assignees

@seanbudd seanbudd

@SaschaCowley SaschaCowley

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. merge-early Merge Early in a developer cycle

Projects

None yet

Milestone

2026.2

Development

Successfully merging this pull request may close these issues.

Magnifier - Text caret is not followed in NVDA Python console

6 participants

@Boumtchack @CyrilleB79 @LeonarddeR @seanbudd @SaschaCowley